-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Performance improvement: Type cast properties on demand #3089
Conversation
Nice!! This seems like a really good optimization target. Excellent work with that profiler. 😃 I have a small idea for how to make the code a little more centralized and perhaps more maintainable: define a little "dict-like" wrapper that is responsible for performing the lazy conversion. Here's a quick sketch I put together: class LazyConvertDict(object):
def __init__(self, data, model_cls):
self.data = data
self.model_cls = model_cls
self._converted = {}
def _convert(self, key, value):
return self.model_cls._type(key).from_sql(value)
def __getitem__(self, key):
if key in self._converted:
return self._converted[key]
elif key in self.data:
value = self._convert(key, self.data[key])
self._converted[key] = value
return value The model class could then just define This would probably need a |
ea09bba
to
5eced8d
Compare
Uploaded a raw version that uses your suggested LazyConvertDict. Testing here looks fine. Performance for some reason seems to be even better with that version, not entirely sure why. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! Looking great already.
5eced8d
to
c90a270
Compare
c90a270
to
f9f2bdd
Compare
AFAICS everything's fixed up now :) |
OK, this is truly awesome!! We should be maintaining a trophy room somewhere that lists all the performance bottlenecks you have found and defeated. Thank you for all your help along these lines! ✨ |
Performance improvement: Type cast properties on demand
I analyzed where time is spent when querying the library and found that most of it is actually type casting properties after retrieving them from the database. However, this isn't really necessary. We hardly ever need access to all properties, so it makes much more sense to delay the actual casting until a property is accessed.
Taking my previous baseline of
3.66s
for master, doing the delayed type casting reduces this to1.96s
, or down to1.45s
when #2798 is also applied (a bit slower now than my first number with it now no longer being a poc).Our of those remaining ~1.5s, about 0.5s is spent on the actual database queries, and the rest is object creation.
I'm open to ideas for further performance improvements here, but we get firmly into area of micro-optimizations (creating 11000 objects in 1s, we'd need to make every object creation 1/22000s faster to save another 0.5s). At least I didn't find any obvious candidates.